HIVE-29478: FileSinkOperator shouldn't check for isMMTable for everyrow being processed#6341
Conversation
|
@nareshpr : approved, but I've just realized there are a few more occurrences of |
|
yes, i will remove all Map conversion in AcidUtils. |
asf-ci-hive
added
tests pending
tests failed
and removed
tests passed
tests pending
labels
asf-ci-hive
added
tests pending
tests unstable
tests passed
and removed
tests failed
tests pending
tests unstable
labels
|
@abstractdog I had done the changes, can you please review ? |
please take care of the sonarqube warnings (unused imports), other than that, looks good to me |
…row being processed
|
|
Taken care of checkstyle unused import. Got successful green build. |
|
|
||
| public static boolean isTablePropertyTransactional(Properties props) { | ||
| return isTablePropertyTransactional(Maps.fromProperties(props)); | ||
| String resultStr = (String) props.get(hive_metastoreConstants.TABLE_IS_TRANSACTIONAL); |
There was a problem hiding this comment.
FYI:
instead code duplication we could have used signature with BinaryOperator<String> props and then
getParameters()::getOrDefault
properties()::getOrDefault
4 participants
What changes were proposed in this pull request?
conf.isMMTable() shouldn't be invoked for every row being processed. This came up as hotspot in ETL file write flow.
Why are the changes needed?
It improves write performance, specifically properties to map conversion is not required to validate a single property for every row being processed.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Tests were not added, as its performance improvement.